Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds smoke tests for conversation compaction functionality and fixes a logic bug in the auto-compaction check. The changes introduce a test script to validate both manual and auto-compaction scenarios, integrate it into the CI workflow, and correct the compaction trigger logic.
- Adds comprehensive test script for manual and auto-compaction validation
- Fixes auto-compaction logic to properly skip when manual compaction is triggered
- Integrates compaction tests into the GitHub Actions workflow
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| scripts/test_compaction.sh | New smoke test script that validates manual compaction (via trigger prompt) and auto-compaction (via threshold), including JSON structure validation |
| crates/goose/src/agents/agent.rs | Fixes logic bug by adding !is_manual_compact condition to auto-compaction check to prevent both from triggering simultaneously |
| .github/workflows/pr-smoke-test.yml | Adds new compaction test step to the CI workflow with appropriate environment configuration |
Comments suppressed due to low confidence (1)
crates/goose/src/agents/agent.rs:1
- Consider adding a comment explaining why auto-compaction is skipped when manual compaction is triggered. This would clarify that the guard prevents both compaction types from running simultaneously and improve code maintainability.
use std::collections::HashMap;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Tests both manual (trigger prompt) and auto compaction (threshold-based) | ||
|
|
||
| if [ -f .env ]; then | ||
| export $(grep -v '^#' .env | xargs) |
There was a problem hiding this comment.
The export $(grep -v '^#' .env | xargs) command is vulnerable to command injection if the .env file contains malicious content. Consider using a safer approach like set -a; source .env; set +a or iterate through the file with proper validation of variable names and values.
| export $(grep -v '^#' .env | xargs) | |
| set -a; source .env; set +a |
…est-and-fix * 'main' of github.com:block/goose: improve linux tray icon support (#5425) feat: log rotation (#5561) use app.isPackaged instead of checking for node env development (#5465) disable RPM build-ID generation to prevent package conflicts (#5563) Add Diagnostics Info to Q&A and Bug Report Templates (#5565) fix: improve server error messages to include HTTP status code (#5532) improvement: add useful error message when attempting to use unauthenticated cursor-agent (#5300) fix: unblock acp via databricks (#5562) feat: add --output-format json flag to goose run command (#5525)
…est-and-fix * 'main' of github.com:block/goose: Document missing recipe param types (#5584) docs: description required for "Add Extension" in cli (#5573) fix: Add schema-aware numeric coercion for MCP tool arguments (#5478) Add uv for uvx in Justfile (#5581) Keep llm logs in place (#5577) bump to 1.12.0 (#5580) automate more of the release process (#5409) add clippy warning for string_slice (#5422)
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| Ok(Box::pin(async_stream::try_stream! { | ||
| let final_conversation = if !needs_auto_compact { | ||
| let final_conversation = if !needs_auto_compact && !is_manual_compact { |
There was a problem hiding this comment.
The condition !needs_auto_compact && !is_manual_compact is logically equivalent to !(needs_auto_compact || is_manual_compact), which can be simplified. However, given that needs_auto_compact already incorporates the !is_manual_compact check (line 753), this condition could be simplified to just !needs_auto_compact since when is_manual_compact is true, needs_auto_compact will always be false.
| let final_conversation = if !needs_auto_compact && !is_manual_compact { | |
| let final_conversation = if !needs_auto_compact { |
* main: (60 commits) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) Avoid web double write (#5601) fix: gemini flash -> pro for mcp smoke tests (#5574) Manual compaction test and fix (#5568) fix: tidy up claude cli handling (#5594) Remove jetbrains (#5602) feat(githubcopilot): add support for newer Copilot AI Models (#5603) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) feat(providers): add Mistral AI provider (#5009) Listen for ctrl-c during provider request (#5585) Also accept null as description, not just missing (#5589) ...
* main: (31 commits) Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) Avoid web double write (#5601) fix: gemini flash -> pro for mcp smoke tests (#5574) Manual compaction test and fix (#5568) fix: tidy up claude cli handling (#5594) Remove jetbrains (#5602) feat(githubcopilot): add support for newer Copilot AI Models (#5603) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) Add environment subsition for auth blocks (#5439) acp: ToolCallLocations and working cancellation (#5588) ...
* main: (21 commits) differentiate debug/release in cache key (#5613) Unify subrecipe and subagent execution through shared recipe pipeline (#5082) Standardize CLI argument flags and update documentation (#5516) Release 1.13.0 fix: move goosehints/AGENTS.md handling to goose, and out of developer extension (#5575) fix: add standard context menu items to prevent empty right-click menu (#5616) Bump openapi in prepare-release (#5611) docs: add access control section to Developer tutorial (#5615) Token state not showing on load, or after message is finished. (#5606) Change the other location too (#5608) feat(ui): bring back quick launcher (#5144) Support platform tools through CLI (#5570) Avoid web double write (#5601) fix: gemini flash -> pro for mcp smoke tests (#5574) Manual compaction test and fix (#5568) fix: tidy up claude cli handling (#5594) Remove jetbrains (#5602) feat(githubcopilot): add support for newer Copilot AI Models (#5603) fix: customised recipe to yaml string to avoid minininjia parsing error (#5494) Add pending extension indicator to extension panel (#5493) ...
Signed-off-by: fbalicchia <fbalicchia@gmail.com>
Signed-off-by: Blair Allan <Blairallan@icloud.com>
Summary
Fix manual compaction triggering bug + add a smoke test.